-
Notifications
You must be signed in to change notification settings - Fork 0
feat-superadmin-set-org-plan #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Introduce AdminSetPlanRequest and AdminSetPlanResponse protobuf types and register a new OrganizationService RPC AdminSetPlan. The request carries organization_id and plan_id (plan_id must match an OrganizationPlan.ID). The response returns the updated Organization and the applied plan_id. This enables superadmin users to set or change an organization's active plan via the gRPC API. Update the generated .pb.go descriptors and file raw descriptor to include the new messages and service method.
Add AdminSetPlan handler to organizations service to allow superadmins to change an organization's active plan. - Authenticate user and require superadmin privilege. - Validate organization_id and plan_id request fields. - Update organizations.plan via a direct DB Exec. - Re-fetch updated organization row (matching ListOrganizations' row shape) and build database.Organization to convert to proto. - Return updated Organization proto and the applied plan_id. - Surface clear connect errors for unauthenticated, forbidden, invalid arguments and backend failures. This provides an administrative endpoint for plan changes and reuses existing row mapping to ensure consistent response shape.
Add a Set Plan dialog to the superadmin organizations page, enabling admins to view and change an organization's plan from the UI. - Add OuiDialog UI for "Set Plan" with organization and current plan display, a plan select, resources preview, and footer actions (Cancel / Set Plan). - Wire dialog state and selection: setPlanDialogOpen, selectedPlanId, selectedPlanInfo, setPlanLoading, selectedOrgId, selectedOrgName, selectedOrgCurrentPlan, and planSelectItems. - Import SuperadminService client and initialize saClient via useConnectClient for future set-plan operations. - Add manageOrgId ref and cleanup an unused manageCreditsOrgId ref (rename/cleanup related to credits management). - Display formatted resource info (CPU, memory, deployments, VPS, monthly credits) and use existing helpers (prettyPlan, formatBytes, OuiCurrency). These changes provide an interactive way to inspect plan resource details and assign plans to organizations, preparing the codepath for server-side plan update integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds functionality for superadmins to set organization plans through both a backend RPC endpoint and a frontend UI dialog. The implementation includes protobuf definitions for AdminSetPlan request/response messages, a backend handler in the organizations service, and a "Set Plan" dialog on the superadmin organizations page.
Changes:
- Added AdminSetPlan RPC method with request/response protobuf messages
- Implemented backend handler for setting organization plans with superadmin authentication
- Created frontend UI dialog for selecting and previewing plan resources before assignment
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/proto/proto/obiente/cloud/organizations/v1/organization_service.proto | Added AdminSetPlanRequest and AdminSetPlanResponse message definitions and RPC method declaration |
| packages/proto/src/generated/obiente/cloud/organizations/v1/organization_service_pb.ts | Generated TypeScript bindings for new protobuf messages and service method |
| apps/shared/proto/obiente/cloud/organizations/v1/organization_service.pb.go | Generated Go bindings for new protobuf messages |
| apps/shared/proto/obiente/cloud/organizations/v1/organizationsv1connect/organization_service.connect.go | Generated Connect RPC client/server stubs for AdminSetPlan method |
| apps/organizations-service/internal/service/service.go | Implemented AdminSetPlan handler with authentication, validation, and database operations |
| apps/dashboard/app/pages/superadmin/organizations/index.vue | Added Set Plan dialog UI with plan selection, resource preview, and API integration |
| apps/dashboard/nuxt.config.ts | Updated API host configuration and Nuxt compatibility version (unrelated to main feature) |
| CLAUDE.md | Added project documentation file (unrelated to main feature) |
| packages/proto/src/generated/obiente/cloud/deployments/v1/deployment_service_pb.ts | Updated health check type comment (unrelated to main feature) |
| apps/shared/proto/obiente/cloud/deployments/v1/deployment_service.pb.go | Updated health check type comment (unrelated to main feature) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Update the organization's plan in the database | ||
| result := database.DB.Exec(` | ||
| UPDATE organizations | ||
| SET plan = ? | ||
| WHERE id = ? | ||
| `, planID, orgID) | ||
| if result.Error != nil { | ||
| return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to update organization plan: %w", result.Error)) |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical bug: This updates the wrong database field. The implementation updates organizations.plan, but the actual plan assignment is managed through the org_quotas table's plan_id field. This is evident from:
EnsurePlanAssignedfunction which updatesOrgQuota.PlanID(apps/shared/pkg/services/organizations/plan_assignment.go:68-85)organizationToProtofunction which loads plan info fromorg_quotastable (line 1504-1543 in this file)
The correct approach is to update the org_quotas.plan_id field, similar to how EnsurePlanAssigned does it. Consider:
var quota database.OrgQuota
err := database.DB.First("a, "organization_id = ?", orgID).Error
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
// Create new quota if it doesn't exist
quota = database.OrgQuota{OrganizationID: orgID, PlanID: planID}
err = database.DB.Create("a).Error
}
// handle error
} else {
quota.PlanID = planID
err = database.DB.Save("a).Error
}
| // Update the organization's plan in the database | |
| result := database.DB.Exec(` | |
| UPDATE organizations | |
| SET plan = ? | |
| WHERE id = ? | |
| `, planID, orgID) | |
| if result.Error != nil { | |
| return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to update organization plan: %w", result.Error)) | |
| // Update the organization's plan via org_quotas (source of truth for plan assignment) | |
| var quota database.OrgQuota | |
| err = database.DB.First("a, "organization_id = ?", orgID).Error | |
| if err != nil { | |
| if errors.Is(err, gorm.ErrRecordNotFound) { | |
| quota = database.OrgQuota{ | |
| OrganizationID: orgID, | |
| PlanID: planID, | |
| } | |
| if err = database.DB.Create("a).Error; err != nil { | |
| return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to create organization quota: %w", err)) | |
| } | |
| } else { | |
| return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to load organization quota: %w", err)) | |
| } | |
| } else { | |
| quota.PlanID = planID | |
| if err = database.DB.Save("a).Error; err != nil { | |
| return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to update organization quota: %w", err)) | |
| } |
| // Ensure the user is a superadmin | ||
| if !auth.IsSuperadmin(ctx, user) { | ||
| return nil, connect.NewError(connect.CodePermissionDenied, fmt.Errorf("permission denied")) |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect permission check: The function uses auth.IsSuperadmin(ctx, user) but other similar admin functions in this service use auth.HasSuperadminPermission(ctx, user, "permission_name") for fine-grained permission checks. For example, AdminAddCredits (line 1771) and AdminRemoveCredits (line 1842) use auth.HasSuperadminPermission.
For consistency and better access control, consider using a specific permission like:
if !auth.HasSuperadminPermission(ctx, user, "organization.admin.set_plan") {
return nil, connect.NewError(connect.CodePermissionDenied, fmt.Errorf("superadmin access required"))
}
| // Ensure the user is a superadmin | |
| if !auth.IsSuperadmin(ctx, user) { | |
| return nil, connect.NewError(connect.CodePermissionDenied, fmt.Errorf("permission denied")) | |
| // Ensure the user has the required superadmin permission | |
| if !auth.HasSuperadminPermission(ctx, user, "organization.admin.set_plan") { | |
| return nil, connect.NewError(connect.CodePermissionDenied, fmt.Errorf("superadmin access required")) |
| future: { | ||
| compatibilityVersion: 4 | ||
| }, |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This configuration change (setting compatibilityVersion: 4) appears unrelated to the stated purpose of this PR. Changing the Nuxt compatibility version is a significant framework upgrade that could have wide-ranging effects on the application behavior and should be:
- Thoroughly tested across the entire application
- Documented in the PR description
- Ideally handled in a separate PR focused on the framework upgrade
If this change is intentional and necessary for the set-plan feature, please explain why in the PR description. Otherwise, it should be reverted.
| # CLAUDE.md | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. | ||
|
|
||
| ## Project Overview | ||
|
|
||
| Obiente Cloud is a distributed Platform-as-a-Service (PaaS) built as an Nx monorepo with 14 Go microservices and a Nuxt 4 dashboard. Services communicate via ConnectRPC (protocol buffers). Deployed on Docker Swarm. | ||
|
|
||
| ## Build & Development Commands | ||
|
|
||
| ### Package Management | ||
| ```bash | ||
| pnpm install # Install all JS/TS dependencies | ||
| ``` | ||
|
|
||
| ### Dashboard (Nuxt 4 frontend) | ||
| ```bash | ||
| nx serve dashboard # Dev server on port 3000 | ||
| nx nuxt:build dashboard # Production build | ||
| nx lint dashboard # ESLint | ||
| nx typecheck dashboard # Type checking | ||
| ``` | ||
|
|
||
| ### Go Services | ||
| ```bash | ||
| cd apps/<service-name> | ||
| go run main.go # Run locally | ||
| go build # Build binary | ||
| go test ./... # Run tests | ||
| ``` | ||
|
|
||
| ### Protocol Buffers | ||
| ```bash | ||
| cd packages/proto | ||
| pnpm build # Regenerate all proto code (buf generate) | ||
| ``` | ||
| Generated Go code goes to `apps/shared/proto/`, TypeScript to `packages/proto/src/generated/`. | ||
|
|
||
| ### Docker | ||
| ```bash | ||
| docker compose up -d # Local dev (all services) | ||
| docker build -f apps/<svc>/Dockerfile -t ghcr.io/obiente/cloud-<svc>:latest . # Build image | ||
| ./scripts/deploy-swarm-dev.sh # Swarm dev deploy | ||
| ./scripts/deploy-swarm-dev.sh -b # Build + deploy | ||
| ``` | ||
|
|
||
| ### Nx | ||
| Always prefer running tasks through `nx` rather than underlying tooling directly. Use `nx run`, `nx run-many`, `nx affected`. | ||
|
|
||
| ## Architecture | ||
|
|
||
| ### Service Ports | ||
| | Service | Port | | ||
| |---------|------| | ||
| | Dashboard | 3000 | | ||
| | API Gateway | 3001 | | ||
| | Auth | 3002 | | ||
| | Organizations | 3003 | | ||
| | Billing | 3004 | | ||
| | Deployments | 3005 | | ||
| | GameServers | 3006 | | ||
| | Orchestrator | 3007 | | ||
| | VPS | 3008 | | ||
| | Support | 3009 | | ||
| | Audit | 3010 | | ||
| | Superadmin | 3011 | | ||
| | Notifications | 3012 | | ||
| | DNS | 8053 | | ||
|
|
||
| ### Key Architectural Patterns | ||
|
|
||
| - **API Gateway** routes all external requests to backend services. Supports both direct service routing and Traefik-based routing. | ||
| - **ConnectRPC** is used for all inter-service communication. Proto definitions live in `packages/proto/proto/obiente/cloud/`. Buf generates both Go and TypeScript clients. | ||
| - **Go workspace** (`go.work`) links all 15 Go modules. Shared code is in `apps/shared/` with packages for auth, database, docker, middleware, orchestrator, quota, etc. | ||
| - **Auth** is handled via Zitadel integration with RBAC. The auth-service validates tokens and manages roles/permissions. | ||
| - **Orchestrator** handles intelligent node selection and load balancing across the Docker Swarm cluster. | ||
| - **Database**: PostgreSQL (primary), TimescaleDB (metrics/audit), Redis (cache, build logs). | ||
| - **Dashboard** uses Nuxt 4, Vue 3, Tailwind CSS v4, Pinia for state, Ark UI for components, and `@connectrpc/connect-web` for API calls. | ||
|
|
||
| ### Monorepo Structure | ||
| - `apps/` - All microservices + dashboard | ||
| - `packages/proto/` - Protobuf definitions and generated code | ||
| - `packages/database/` - Drizzle ORM schemas and migrations | ||
| - `packages/config/` - Shared ESLint, Prettier, TypeScript configs | ||
| - `packages/types/` - Shared TypeScript types | ||
| - `tools/nxsh/` - Custom Nx shell executor | ||
| - `monitoring/` - Prometheus & Grafana configs | ||
| - `scripts/` - Deployment and operational scripts | ||
|
|
||
| ### Docker Compose Files | ||
| - `docker-compose.yml` - Local development | ||
| - `docker-compose.base.yml` - Shared env vars (YAML anchors) | ||
| - `docker-compose.swarm.yml` - Production swarm | ||
| - `docker-compose.swarm.dev.yml` - Dev swarm (must use `docker stack deploy`, not `docker compose`) | ||
| - `docker-compose.swarm.ha.yml` - HA production with PostgreSQL cluster |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire CLAUDE.md file addition appears unrelated to this PR's stated purpose of adding superadmin set-plan functionality. This is a comprehensive documentation file about the project structure and development practices.
While this documentation may be valuable, it should be added in a separate PR dedicated to documentation improvements, not bundled with a feature implementation PR. This makes the PR harder to review and mixes unrelated concerns.
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.cpuCores || '∞' }} cores</OuiText> | ||
| <OuiText size="sm" color="secondary">Memory</OuiText> | ||
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.memoryBytes ? formatBytes(Number(selectedPlanInfo.memoryBytes)) : '∞' }}</OuiText> | ||
| <OuiText size="sm" color="secondary">Deployments</OuiText> | ||
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.deploymentsMax || '∞' }}</OuiText> | ||
| <OuiText size="sm" color="secondary">VPS Instances</OuiText> | ||
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.maxVpsInstances || '∞' }}</OuiText> |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential ambiguity in zero value display: The code displays 0 values as '∞' (infinity) for CPU cores, deployments, and VPS instances (e.g., line 162: selectedPlanInfo.cpuCores || '∞'). However, in the database schema, 0 is used to represent "unlimited" for some fields (e.g., MaxVpsInstances in OrganizationPlan model), but for other fields like CPUCores, 0 likely means "none" rather than "unlimited".
This could confuse users if a plan actually has 0 CPU cores (which would be invalid) vs unlimited. Consider:
- Using a more explicit check like
selectedPlanInfo.cpuCores === 0 || selectedPlanInfo.cpuCores === null ? '∞' : selectedPlanInfo.cpuCores - Or displaying 0 as 0 and using -1 or null to represent unlimited
- Adding validation to ensure critical resources like CPU can't be 0
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.cpuCores || '∞' }} cores</OuiText> | |
| <OuiText size="sm" color="secondary">Memory</OuiText> | |
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.memoryBytes ? formatBytes(Number(selectedPlanInfo.memoryBytes)) : '∞' }}</OuiText> | |
| <OuiText size="sm" color="secondary">Deployments</OuiText> | |
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.deploymentsMax || '∞' }}</OuiText> | |
| <OuiText size="sm" color="secondary">VPS Instances</OuiText> | |
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.maxVpsInstances || '∞' }}</OuiText> | |
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.cpuCores === 0 || selectedPlanInfo.cpuCores == null ? '∞' : selectedPlanInfo.cpuCores }} cores</OuiText> | |
| <OuiText size="sm" color="secondary">Memory</OuiText> | |
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.memoryBytes === 0 || selectedPlanInfo.memoryBytes == null ? '∞' : formatBytes(Number(selectedPlanInfo.memoryBytes)) }}</OuiText> | |
| <OuiText size="sm" color="secondary">Deployments</OuiText> | |
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.deploymentsMax === 0 || selectedPlanInfo.deploymentsMax == null ? '∞' : selectedPlanInfo.deploymentsMax }}</OuiText> | |
| <OuiText size="sm" color="secondary">VPS Instances</OuiText> | |
| <OuiText size="sm" class="font-mono text-right">{{ selectedPlanInfo.maxVpsInstances === 0 || selectedPlanInfo.maxVpsInstances == null ? '∞' : selectedPlanInfo.maxVpsInstances }}</OuiText> |
| if orgID == "" || planID == "" { | ||
| return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("organization_id and plan_id are required")) | ||
| } | ||
|
|
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing input validation: The plan_id should be validated to ensure it corresponds to an actual plan in the organization_plans table before attempting the update. Without this check, the system could be configured with an invalid plan ID that will cause downstream errors when trying to load plan information.
Add validation similar to what's in EnsurePlanAssigned:
var plan database.OrganizationPlan
if err := database.DB.First(&plan, "id = ?", planID).Error; err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("plan not found: %s", planID))
}
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to validate plan: %w", err))
}
| // Validate that the plan exists | |
| var plan database.OrganizationPlan | |
| if err := database.DB.First(&plan, "id = ?", planID).Error; err != nil { | |
| if errors.Is(err, gorm.ErrRecordNotFound) { | |
| return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("plan not found: %s", planID)) | |
| } | |
| return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to validate plan: %w", err)) | |
| } |
| `, planID, orgID) | ||
| if result.Error != nil { | ||
| return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to update organization plan: %w", result.Error)) | ||
| } |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error check on database result: The code doesn't verify that the UPDATE actually affected any rows. If the organization_id doesn't exist, the Exec will succeed silently without updating anything. This could lead to confusing behavior where the API returns success but no change occurred.
Add a check like:
if result.RowsAffected == 0 {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("organization not found"))
}
| } | |
| } | |
| if result.RowsAffected == 0 { | |
| return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("organization not found")) | |
| } |
| // When running locally (not in Docker), use localhost with Traefik port | ||
| // When running in Docker, use api-gateway service name | ||
| apiHostInternal: process.env.NUXT_API_HOST_INTERNAL || process.env.NUXT_PUBLIC_API_HOST || "http://localhost:80", | ||
| apiHostInternal: process.env.NUXT_API_HOST_INTERNAL || process.env.NUXT_PUBLIC_API_HOST || "http://api.localhost", |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This configuration change appears unrelated to the stated purpose of this PR (adding superadmin set-plan functionality). The apiHostInternal URL is changed from http://localhost:80 to http://api.localhost, which is a significant infrastructure change that could affect local development routing.
If this change is intentional and necessary, it should be explained in the PR description or split into a separate PR. Otherwise, it should be reverted to keep this PR focused on the set-plan feature.
|
|
||
| const ( | ||
| HealthCheckType_HEALTHCHECK_TYPE_UNSPECIFIED HealthCheckType = 0 // No health check | ||
| HealthCheckType_HEALTHCHECK_TYPE_UNSPECIFIED HealthCheckType = 0 // Auto-detect (TCP if routing exists, otherwise no healthcheck) |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment update appears unrelated to the stated purpose of this PR (adding superadmin set-plan functionality). The change clarifies the behavior of HEALTHCHECK_TYPE_UNSPECIFIED, but belongs in a separate documentation or deployment service PR.
| // Admin: Set the active plan for an organization (superadmin only) | ||
| rpc AdminSetPlan(AdminSetPlanRequest) returns (AdminSetPlanResponse); |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method placement inconsistency: The AdminSetPlan RPC is placed at the beginning of the service definition (line 13), while other admin methods like AdminAddCredits and AdminRemoveCredits are grouped together later (around lines 61-64 in the original file). For consistency and better organization, consider moving AdminSetPlan to be grouped with the other admin methods.
Summary
Motivation
Changes